Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Class loading deadlock between PermalinkProjectAction.Permalink & PeepholePermalink #8736

Merged
merged 7 commits into from
Dec 9, 2023
Merged

Class loading deadlock between PermalinkProjectAction.Permalink & PeepholePermalink #8736

merged 7 commits into from
Dec 9, 2023
+234 −106

Conversation

jglick
Copy link
Member

@jglick jglick commented Dec 1, 2023 •
edited
Loading

A functional test in CloudBees CI was observed to hang occasionally. Inspecting the thread dump revealed an apparent class loading deadlock. It is unknown whether this would affect production scenarios.

Fundamentally the problem is the use of static final fields for non-constant expressions. It would be incompatible to just delete these fields, unfortunately; plugins using them include for example

  • build-symlink
  • build-alias-setter
  • copyartifact
  • run-selector
  • infradna-backup (CloudBees CI)

As noted here it is not always clear how to solve these problems compatibly.

Testing done

Test usually deadlocks. Thread dump after 7dee8b2

"main" #1 prio=5 os_prio=0 cpu=350.37ms elapsed=13.59s tid=0x00007f9848016000 nid=0x958f in Object.wait()  [0x00007f984f0b3000]
   java.lang.Thread.State: RUNNABLE
	at jenkins.model.PeepholePermalinkTest.classLoadingDeadlock(PeepholePermalinkTest.java:42)
	at …

"Thread-1" #16 prio=5 os_prio=0 cpu=0.99ms elapsed=13.17s tid=0x00007f9848b62800 nid=0x95ab in Object.wait()  [0x00007f981c8f4000]
   java.lang.Thread.State: RUNNABLE
	at hudson.model.PermalinkProjectAction$Permalink.<clinit>(PermalinkProjectAction.java:114)
	at jenkins.model.PeepholePermalinkTest.lambda$classLoadingDeadlock$0(PeepholePermalinkTest.java:39)
	at jenkins.model.PeepholePermalinkTest$$Lambda$328/0x000000084010d040.run(Unknown Source)
	at java.lang.Thread.run([email protected]/Thread.java:829)

Proposed changelog entries

  • Preventing a deadlock that can occur when loading PermalinkProjectAction.Permalink.

Before the changes are marked as ready-for-merge:

Maintainer checklist
Beta Give feedback

@jglick

This comment was marked as outdated.

Sign in to view
@jglick

This comment was marked as outdated.

Sign in to view
@jglick jglick changed the title Class loading deadlock between PermalinkProjectAction & PeepholePermalink Class loading deadlock between PermalinkProjectAction.Permalink & PeepholePermalink Dec 1, 2023
@jglick jglick marked this pull request as ready for review December 1, 2023 19:39
@jglick jglick requested a review from Vlatombe December 1, 2023 19:39
@NotMyFault NotMyFault added the bug For changelog: Minor bug. Will be listed after features label Dec 7, 2023
@NotMyFault NotMyFault requested a review from a team December 7, 2023 21:24
timja
timja approved these changes Dec 8, 2023
View reviewed changes
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/label ready-for-merge

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Dec 8, 2023
@NotMyFault NotMyFault merged commit f9a777b into jenkinsci:master Dec 9, 2023
17 checks passed
@jglick jglick deleted the PeepholePermalinkTest branch December 11, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timja timja timja approved these changes

@Vlatombe Vlatombe Vlatombe approved these changes

Assignees
No one assigned
Labels
bug For changelog: Minor bug. Will be listed after features ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jglick @Vlatombe @timja @NotMyFault